Guard MarshalByRefObject, [Serializable], and InitializeLifetimeService behind #if NETFRAMEWORK#8174
Merged
Conversation
…ce behind #if NETFRAMEWORK - Make all MarshalByRefObject inheritance conditional on NETFRAMEWORK across 10 classes - Wrap InitializeLifetimeService overrides and [SecurityCritical] in #if NETFRAMEWORK - Guard [Serializable] with #if NETFRAMEWORK on 11 non-exception data types - Conditionally include 'using System.Security' only for NETFRAMEWORK - Exception types retain [Serializable] unconditionally (standard .NET convention) On modern .NET (net8.0/net9.0), these classes are now plain types with no remoting or BinaryFormatter baggage. On net462, the full AppDomain marshaling support is preserved.
Contributor
There was a problem hiding this comment.
Pull request overview
This PR scopes .NET Framework remoting/serialization artifacts (MarshalByRefObject, [Serializable], and InitializeLifetimeService) to #if NETFRAMEWORK, keeping AppDomain marshaling behavior on net462 while removing remoting-related baggage from modern .NET builds.
Changes:
- Guarded
MarshalByRefObjectinheritance andInitializeLifetimeService()overrides behind#if NETFRAMEWORK. - Guarded
[Serializable]on non-exception transport/data types behind#if NETFRAMEWORK. - Conditionally included
using System.Security;only where[SecurityCritical]is compiled to avoid unused-using warnings.
Show a summary per file
| File | Description |
|---|---|
| src/TestFramework/TestFramework/Attributes/TestMethod/TestResult.cs | Makes TestResult [Serializable] only on .NET Framework. |
| src/Adapter/MSTestAdapter.PlatformServices/RecursiveDirectoryPath.cs | Guards [Serializable], MarshalByRefObject, InitializeLifetimeService, and System.Security usage to .NET Framework. |
| src/Adapter/MSTestAdapter.PlatformServices/ObjectModel/UnitTestElement.cs | Guards [Serializable] to .NET Framework. |
| src/Adapter/MSTestAdapter.PlatformServices/ObjectModel/TestMethod.cs | Guards [Serializable] to .NET Framework. |
| src/Adapter/MSTestAdapter.PlatformServices/ObjectModel/TestAssemblySettings.cs | Guards [Serializable] to .NET Framework. |
| src/Adapter/MSTestAdapter.PlatformServices/ObjectModel/StackTraceInformation.cs | Guards [Serializable] to .NET Framework. |
| src/Adapter/MSTestAdapter.PlatformServices/NopTraceLogger.cs | Makes MarshalByRefObject base conditional for .NET Framework remoting. |
| src/Adapter/MSTestAdapter.PlatformServices/MSTestSettings.cs | Guards [Serializable] to .NET Framework. |
| src/Adapter/MSTestAdapter.PlatformServices/Helpers/ReflectHelper.cs | Makes MarshalByRefObject/InitializeLifetimeService and System.Security usage conditional on .NET Framework. |
| src/Adapter/MSTestAdapter.PlatformServices/Execution/UnitTestRunner.cs | Makes MarshalByRefObject/InitializeLifetimeService and System.Security usage conditional on .NET Framework. |
| src/Adapter/MSTestAdapter.PlatformServices/Execution/TypeCache.cs | Makes MarshalByRefObject/InitializeLifetimeService and System.Security usage conditional on .NET Framework. |
| src/Adapter/MSTestAdapter.PlatformServices/Execution/TestExecutionManager.cs | Makes nested RemotingMessageLogger inherit MarshalByRefObject only on .NET Framework. |
| src/Adapter/MSTestAdapter.PlatformServices/Execution/TestAssemblySettingsProvider.cs | Makes MarshalByRefObject/InitializeLifetimeService and System.Security usage conditional on .NET Framework. |
| src/Adapter/MSTestAdapter.PlatformServices/Discovery/AssemblyEnumerator.cs | Makes MarshalByRefObject/InitializeLifetimeService and System.Security usage conditional on .NET Framework. |
| src/Adapter/MSTestAdapter.PlatformServices/Discovery/AssemblyEnumerationResult.cs | Guards [Serializable] to .NET Framework. |
| src/Adapter/MSTestAdapter.PlatformServices/Deployment/TestRunDirectories.cs | Guards [Serializable] to .NET Framework. |
| src/Adapter/MSTestAdapter.PlatformServices/Deployment/DeploymentItem.cs | Guards [Serializable] on enum and class to .NET Framework. |
| src/Adapter/MSTest.TestAdapter/VSTestAdapter/EqtTraceLogger.cs | Makes MarshalByRefObject base conditional for .NET Framework remoting. |
| src/Adapter/MSTest.TestAdapter/TestingPlatformAdapter/MTPTraceLogger.cs | Makes MarshalByRefObject base conditional for .NET Framework remoting. |
Copilot's findings
- Files reviewed: 19/19 changed files
- Comments generated: 0
Youssef1313
reviewed
May 13, 2026
Member
Author
|
@copilot resolve the merge conflicts in this pull request |
Co-authored-by: Evangelink <11340282+Evangelink@users.noreply.github.com>
Contributor
Resolved. The only conflict was in |
Removing [Serializable] for non-NETFRAMEWORK exposed an IDE0032 false positive on s_currentSettings/s_runConfigurationSettings, whose backing properties use '??=' lazy initialization (not expressible as an auto-property). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Youssef1313
reviewed
May 14, 2026
- Replace IDE0032 suppression in MSTestSettings with C# 'field' keyword pattern (matches PlatformServiceProvider/SettingsProvider style) - Change InitializeLifetimeService overrides from 'object => null!' to 'object? => null' to match the established pattern in AssemblyResolver/AssemblyLoadWorker, removing the null-forgiving operator Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
These attributes are no-ops on modern .NET since TestMethod is no longer [Serializable] there. Wrap them in #if NETFRAMEWORK for consistency with the rest of the PR. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…#if NETFRAMEWORK Same consistency fix as the previous commit on TestMethod: the [field: NonSerialized] attribute is a no-op on modern .NET since [Serializable] is now scoped to NETFRAMEWORK. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Update stale comment in TestResult.cs to clarify cross-AppDomain only applies on .NET Framework - Add comment in AssemblyEnumerator.cs explaining why System.Runtime.Serialization using is unconditional Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Audit and clean up
MarshalByRefObjectand[Serializable]usage acrosssrc/to ensure these .NET Framework remoting artifacts are properly scoped tonet462only.Changes
MarshalByRefObjectinheritance (10 classes)Made all
MarshalByRefObjectbase class inheritance conditional on#if NETFRAMEWORK:Classes with only MarshalByRefObject as base (6):
AssemblyEnumerator,ReflectHelper,RecursiveDirectoryPath,UnitTestRunner,TypeCache,TestAssemblySettingsProviderClasses implementing MarshalByRefObject + an interface (4):
RemotingMessageLogger,NopTraceLogger,EqtTraceLogger,MTPTraceLoggerInitializeLifetimeServiceoverrides (6 methods)Wrapped all
InitializeLifetimeService()overrides (including[SecurityCritical]and the previous[Obsolete]attributes) entirely in#if NETFRAMEWORK. Onnet462, the override returnsnullfor infinite lifetime. On modern .NET, the override doesn't exist since the base class is no longerMarshalByRefObject.Also conditionally included
using System.Security;to avoid IDE0005 warnings.[Serializable]attribute (11 non-exception types)Guarded
[Serializable]with#if NETFRAMEWORKon data types that only needed it forBinaryFormattermarshaling across AppDomain boundaries:RecursiveDirectoryPath,UnitTestElement,TestMethod,TestAssemblySettings,StackTraceInformation,MSTestSettings,AssemblyEnumerationResult,TestRunDirectories,DeploymentItem,DeploymentItemOriginType,TestResultException types (
AssertFailedException,UnitTestAssertException, etc.) intentionally retain[Serializable]unconditionally — this is a standard .NET convention for exceptions.Already correctly guarded (no changes needed)
These were already behind
#if NETFRAMEWORK:AssemblyLoadWorker,StaticStateHelper(inAppDomainUtilities.cs),AssemblyResolver(conditional base class)AppDomain.CreateDomain/Unloadcalls inAppDomainUtilities.cs,AppDomainWrapper.cs,IAppDomain.cs,TestSourceHost.csResult
On modern .NET (
net8.0/net9.0), these classes are now plain types with no remoting orBinaryFormatterbaggage. Onnet462, the full AppDomain marshaling support is preserved.Validation
dotnet buildnet9.0 — 0 warnings, 0 errorsdotnet buildnet462 — 0 warnings, 0 errorsdotnet buildMSTest.TestAdapter net9.0 — 0 warnings, 0 errorsdotnet buildTestFramework net9.0 — 0 warnings, 0 errors